-
Notifications
You must be signed in to change notification settings - Fork 646
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prevent MoveBucket
from moving a bucket across two different db files
#671
Conversation
c1837ee
to
82c3487
Compare
Let's add 3 cases below,
|
MoveBucket
from moving a bucket across two different db files
c055ad5
to
ff0b03b
Compare
done 👍🏽 |
ff0b03b
to
8f4afad
Compare
@Elbehery |
8f4afad
to
b64adb8
Compare
fixed .. my bad, forgot to close the second db file :'( deep apologies |
b64adb8
to
b96ff9c
Compare
i got it, i used the wrong var to in the defer :'( |
b96ff9c
to
f905652
Compare
f905652
to
8d2ef62
Compare
@Elbehery Please let's try to get this PR merged today before I create the tag 1.4.0-alpha.0 |
8d2ef62
to
1e43c74
Compare
@ahrtr updated & rebased 👍🏽 |
movebucket_test.go
Outdated
err = dstDB.Update(func(tx *bbolt.Tx) error { | ||
dstBucket := prepareBuckets(t, tx, dstBucketPath...) | ||
mErr := srcBucket.MoveBucket([]byte(bucketToMove), dstBucket) | ||
require.Equal(t, expectedErr, mErr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor comment:
require.Equal(t, expectedErr, mErr) | |
require.Equal(t, errors.ErrDifferentDB, mErr) |
movebucket_test.go
Outdated
srcBucketPath := []string{"sb1", "sb2"} | ||
dstBucketPath := []string{"db1", "db2"} | ||
bucketToMove := "bucketToMove" | ||
expectedErr := errors.ErrDifferentDB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
expectedErr := errors.ErrDifferentDB |
Signed-off-by: Mustafa Elbehery <melbeher@redhat.com>
1e43c74
to
7555f26
Compare
@ahrtr updated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Thanks
This PR adds more test to move bucket feature
Refer to #635 (comment)
cc @ahrtr @fuweid